Skip to content

Remove explicit SSE content encoding#1598

Closed
DragonFSKY wants to merge 1 commit into
modelcontextprotocol:mainfrom
DragonFSKY:fix/1574-sse-content-encoding
Closed

Remove explicit SSE content encoding#1598
DragonFSKY wants to merge 1 commit into
modelcontextprotocol:mainfrom
DragonFSKY:fix/1574-sse-content-encoding

Conversation

@DragonFSKY
Copy link
Copy Markdown
Contributor

@DragonFSKY DragonFSKY commented May 24, 2026

Motivation and Context

Fixes #1574.

SSE responses currently emit Content-Encoding: identity even though no content coding is applied. RFC 9110 section 8.4 defines Content-Encoding as the list of content codings that were applied to the representation and notes that identity is reserved for Accept-Encoding, so it SHOULD NOT be included in Content-Encoding.

Since no SSE content coding is applied here, the response can omit the header while still preserving the existing SSE buffering controls.

Changes

  • Removes the explicit Content-Encoding: identity assignment from SSE response initialization.
  • Updates legacy SSE and Streamable HTTP integration tests to assert that SSE responses do not include a content encoding.
  • Keeps the existing Cache-Control, X-Accel-Buffering, and DisableBuffering() behavior.

How Has This Been Tested?

  • dotnet test tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj -f net10.0 --filter "FullyQualifiedName~EventSourceResponse_Includes_ExpectedHeaders"
    • RED before the production change: 3 failed because the collection contained identity.
    • GREEN after the production change: 3 passed.
  • dotnet test tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj -f net10.0 --no-build --no-restore
    • 377 passed, 18 skipped.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository style guidelines
  • New and existing relevant tests pass locally
  • I have added appropriate regression coverage
  • Documentation updates were not needed for this header-only behavior fix

Signed-off-by: Dongliang Xie <dragonfsky@gmail.com>
Copy link
Copy Markdown
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I shouldn't have hastily hit approve here.

Thanks for the spec violation catch, but this header is intentional. ASP.NET Core's own ServerSentEventsServerTransport and Razor Components SSR both still set Content-Encoding: identity to suppress IIS's native DynamicCompressionModule (and as a defense against ResponseCompressionMiddleware if a user adds text/event-stream to their MIME list). IHttpResponseBodyFeature.DisableBuffering() only disables Kestrel-level buffering. it doesn't reach IIS's native compression module, which can sit in front of in-process ANCM workers and will buffer chunks to gzip them, silently breaking SSE streaming. Closing this in favor of keeping the existing behavior. Happy to revisit if you have a concrete client that rejects the header.

  1. SignalR Core's ServerSentEventsServerTransport.cs (dotnet/aspnetcore main):**
context.Response.ContentType = "text/event-stream";
context.Response.Headers.CacheControl = "no-cache,no-store";
context.Response.Headers.Pragma = "no-cache";

// Make sure we disable all response buffering for SSE
var bufferingFeature = context.Features.GetRequiredFeature<IHttpResponseBodyFeature>();
bufferingFeature.DisableBuffering();

context.Response.Headers.ContentEncoding = "identity";

It still sets the header after DisableBuffering(). They're treated as solving different problems.
src/SignalR/common/Http.Connections/src/Internal/Transports/ServerSentEventsServerTransport.cs

  1. Razor Components streaming SSR does the same thing, with an explicit comment naming IIS:
// An incomplete QuiescenceTask indicates there may be streaming rendering updates.
// Disable all response buffering and compression on IIS like SignalR's ServerSentEventsServerTransport does.
var bufferingFeature = context.Features.GetRequiredFeature<IHttpResponseBodyFeature>();
bufferingFeature.DisableBuffering();
// ...
context.Response.Headers.ContentEncoding = "identity";

What DisableBuffering() does and doesn't cover

Layer DisableBuffering() X-Accel-Buffering: no Content-Encoding: identity
Kestrel response body
nginx reverse proxy partial
IIS DynamicCompressionModule (out-of-process or in-process via ANCM)
ASP.NET Core ResponseCompressionMiddleware

The IIS native DynamicCompressionModule runs in the IIS pipeline regardless of ANCM mode, and it skips responses that already have a Content-Encoding header (because they look "already encoded"). When it doesn't skip, it gzips chunks before flushing, which silently breaks SSE streaming. That's the failure mode the header is defending against.

RFC 9110 vs. pragmatics

PR #1598's RFC 9110 §8.4 argument is technically correct (identity is reserved for Accept-Encoding and SHOULD NOT appear in Content-Encoding), but this is exactly the kind of "the wire format is slightly impure to defeat misbehaving intermediaries" choice that the rest of ASP.NET Core has decided is worth it. SignalR Core hasn't removed it despite the same RFC text existing for years.

@halter73 halter73 closed this Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid Content-Encoding: identity header in SSE responses (RFC 9110 violation)

3 participants